Skip to content

Fixed race condition when passing data to bgfx. Issue #1398#1628

Merged
bkaradzic-microsoft merged 2 commits intomasterfrom
fix-1398
Apr 11, 2026
Merged

Fixed race condition when passing data to bgfx. Issue #1398#1628
bkaradzic-microsoft merged 2 commits intomasterfrom
fix-1398

Conversation

@bkaradzic-microsoft
Copy link
Copy Markdown
Contributor

Change makes image pointer not being touched after passing data to bgfx.

Alternative solution is to use bgfx::copy instead of bgfx::makeRef.

Copilot AI review requested due to automatic review settings March 12, 2026 03:13
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Addresses issue #1398 by reducing the chance of a use-after-free when uploading texture data to bgfx via bgfx::makeRef, ensuring CPU-side code doesn’t keep reading bimg::ImageContainer fields after bgfx may have triggered the release callback.

Changes:

  • Cache m_numMips before mip iteration in 2D texture uploads to avoid reading from a potentially freed bimg::ImageContainer.
  • Cache m_numMips before mip iteration in cube texture uploads (single-image-per-face path).
  • Modify the cube-texture release callback condition to only free on the last face/last mip.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread Plugins/NativeEngine/Source/NativeEngine.cpp
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread Plugins/NativeEngine/Source/NativeEngine.cpp
Copy link
Copy Markdown
Contributor

@bghgary bghgary left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

discussed offline

@bkaradzic-microsoft bkaradzic-microsoft enabled auto-merge (squash) April 11, 2026 00:03
@bkaradzic-microsoft bkaradzic-microsoft merged commit e8ee5f7 into master Apr 11, 2026
31 checks passed
@bkaradzic-microsoft bkaradzic-microsoft deleted the fix-1398 branch April 11, 2026 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants